Skip to content

TPC TimeSeries: fix silent track loss from binning overflow - #15658

Merged
alcaliva merged 4 commits into
AliceO2Group:devfrom
miranov25:dev
Aug 4, 2026
Merged

TPC TimeSeries: fix silent track loss from binning overflow#15658
alcaliva merged 4 commits into
AliceO2Group:devfrom
miranov25:dev

Conversation

@miranov25

Copy link
Copy Markdown
Contributor

Bin indices for tgl, phi, qPt, and multiplicity were used as implicit track selection cuts: tracks outside histogram range were silently dropped (return). Replace with std::clamp — edge bins become overflow bins (standard ROOT convention). No change for tracks within range.

Bug: changing --max-qPt or --mult-max removed tracks from ALL outputs (DCA, dEdx, etc.), not just the binned histograms.

@miranov25

Copy link
Copy Markdown
Contributor Author

Hello @matthias-kleiner and @shahor02,

I want to commit the code. Please do not merge it yet. I still have to make a build, and we should test it.

As I described in the Mattermost chat, logically, the modified code should perform clipping instead of cutting.

I am building it now.

@alibuild

alibuild commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Error while checking build/O2/fullCI_slc9 for 3c709e1 at 2026-08-02 17:49:

No log files found

Full log here.

@miranov25

Copy link
Copy Markdown
Contributor Author

Hello @shahor02 and all

Are these build errors normal? I assume it is a temporary problem.I built it yesterday on my Mac.

For me, it compiles locally (Mac with a Linux container). I am now trying at GSI with another container.

Now I am rebuilding with ITS and TRD information (not in this branch) , locally and at GSI

@shahor02

shahor02 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Ignore CI, it is broken.

@miranov25

Copy link
Copy Markdown
Contributor Author

Hello @shahor02,

In the meantime, I have updated the time series, adding ITS (cluster shape) and TRD tracklets (native).
I have put them in a separate branch for now, so they are not included in the pull request:
https://github.com/miranov25/AliceO2/tree/timeseries-phase03

My build with these changes has not finished yet, as I have limited space on my Mac (one full O2 build takes 375 GB). I cannot have two builds at once (I need to set up a new disk, as each rebuild is a risk of destroying my working Python setup - which already happened after the update).

I am now building at GSI, where I have enough space for three installations (one working version plus two additional builds).

Should I submit a new pull request and close the original one already?

GSI build can be ready in 2 hours - seeing the progress.

@shahor02

shahor02 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

@miranov25 Why don't you add extra commits here? If you build as
aliBuild -jXXX O2PDPSuite --defaulst o2 --disable O2Physics, then it is much faster and takes <120 GB (at least on ubuntu)

@miranov25

Copy link
Copy Markdown
Contributor Author

@miranov25 Why don't you add extra commits here? If you build as aliBuild -jXXX O2PDPSuite --defaulst o2 --disable O2Physics, then it is much faster and takes <120 GB (at least on ubuntu)

I did not know about that switch. I will try. It will solve part of my problems.

===
I am testing now at GSI - baseline finished. Once finished I can put it to this push request.

ln -s /lustre/alice/tpcdata/Run3/SCDprodTests/fullRec/PbPb2025/closure/wSoftware_2025_12_09/goodRange_fullRun_alignment.v10n/LHC25an.b5p/567939.19kHz/0000/* .

o2-global-track-cluster-reader --shm-segment-size 68719476736 --cluster-types "TPC,TOF,FT0" --disable-mc --track-types "ITS,TPC,ITS-TPC,ITS-TPC-TOF,ITS-TPC-TRD-TOF" --primary-vertices --hbfutils-config o2_tfidinfo.root | o2-tpc-time-series-workflow --enable-unbinned-root-output --shm-segment-size 68719476736 -b

@miranov25

Copy link
Copy Markdown
Contributor Author

Extracted calibrated tracklets have sense, cpu overhead is small, space overhead is ~ 10 % vs reference:

 treeTimeSeries->Draw("trdCalibTracklets.mDy:qpt>>his(100,-3,3,100,-5,5)","(trdPattern>0)&&(nTRDTracklets>3)&&hasITSTPC>0","colz")


treeTimeSeries->Draw("trdTracklets.getSlopeFloat():qpt>>his(100,-3,3,100,-0.2,0.2)","(trdPattern>0)&&(nTRDTracklets>3)&&hasITSTPC>0","colz")

image image

@shahor02 shahor02 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miranov25 please apply clang-format, otherwise, it looks ok

@miranov25

Copy link
Copy Markdown
Contributor Author

I tried to get it but I did not managed.

@miranov25

Copy link
Copy Markdown
Contributor Author

I tried:
git fetch origin
git merge origin/alibuild-cleanup-15658 # or whatever branch PR #8 is from

But nothing happend

miranov25 and others added 3 commits August 3, 2026 17:25
Bin indices for tgl, phi, qPt, and multiplicity were used as implicit
track selection cuts: tracks outside histogram range were silently
dropped (return). Replace with std::clamp — edge bins become overflow
bins (standard ROOT convention). No change for tracks within range.

Bug: changing --max-qPt or --mult-max removed tracks from ALL outputs
(DCA, dEdx, etc.), not just the binned histograms.
…atching

Phase 0.2 — binning overflow fix:
  - Replace bounds-check-and-return with std::clamp on all 4 bin indices
  - Edge bins act as saturated overflow; no tracks silently dropped

Phase 0.3 D1 — ITS cluster sizes (per-track, unbinned):
  - itsClusterSizes: packed 4-bit per layer (bit 28 kSharedClusters masked)
  - itsHasSharedClusters, itsPattern: 7-bit layer hit pattern

Phase 0.3 D2 — TRD tracklet objects (per-track, unbinned):
  - Native Tracklet64[6] and CalibratedTracklet[6] per layer
  - trdPattern (6-bit validity mask), nTRDTracklets
  - requestTRDTracklets added to DataRequest

Phase 0.3 D3 — TRD matching fraction (per-TF):
  - nITSTPCBasedPVContributors, nITSTPCWithTRDPVContributors, fracTRD
  - NaN for zero denominator. ClassDefNV 7 -> 8.
Replace flat primitive arrays with std::vector<Tracklet64> and
std::vector<CalibratedTracklet>. std::array failed ROOT serialization
(missing ShowMember); std::vector with ROOT dictionary works.
@miranov25

Copy link
Copy Markdown
Contributor Author

Clang was fixed
@shahor02

@shahor02 shahor02 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will merge once the full-CI is passed.

@miranov25

Copy link
Copy Markdown
Contributor Author

For the record: AI team Code review request and review summary.

  • some memory/allocation optimization possible (but not needed fr a moment)
  • for tests - we are missing infrastructure

https://claude.ai/chat/42746134-f438-4c46-9702-7bd96d4a8b93:

PHASE_0_3_TimeAI_Closure_CRR.md

PHASE_0_3_timeSeriesAI_Official_Approval_Summary_GPT4_AI_20260804.md

@alcaliva
alcaliva enabled auto-merge (squash) August 4, 2026 01:55
@alcaliva
alcaliva merged commit 907c011 into AliceO2Group:dev Aug 4, 2026
10 of 11 checks passed
@miranov25

Copy link
Copy Markdown
Contributor Author

For the record - and furher crosscheck
Main reviewer for this PHASE:
https://chatgpt.com/g/g-p-6a38d716cec48191847a0be3c7b3b9a3-timeai/c/6a6f0f8e-53ec-83eb-a251-0e9284757997
Test working directory at GSI:
/lustre/alice/users/miranov/NOTESData/alice-tpc-notes/AI/TimeAI/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants